Skip to content

fix(review): use configured model for fix sessions#4091

Open
alex-alecu wants to merge 2 commits into
mainfrom
fix/review-fix-session-model
Open

fix(review): use configured model for fix sessions#4091
alex-alecu wants to merge 2 commits into
mainfrom
fix/review-fix-session-model

Conversation

@alex-alecu

@alex-alecu alex-alecu commented Jun 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • Resolve review fix-session models from the exact GitHub integration row that created the review, matching KiloCode Bot's metadata.model_slug behavior.
  • Fall back to kilo-auto/frontier when the review has no linked integration, the row was deleted, or the stored bot model is invalid.
  • Fail closed when a linked integration row no longer matches the authorized review's platform or user/organization owner, avoiding owner-wide installation lookup.

Verification

N/A — no manual production flow was executed because it requires an authenticated Code Review fix link.

Visual Changes

N/A

Reviewer Notes

Cloud Agent still validates the submitted catalog model in the correct personal or organization context. Historical cloud_agent_code_reviews.model usage data remains reporting-only and is not reused for fix sessions.

@alex-alecu
fix(review): use configured model for fix sessions
3cdd59b 
Resolve the owner catalog model before preparing a review fix session so usage-resolved model IDs are not rejected.
@kilo-code-bot

kilo-code-bot Bot commented Jun 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental rewrite resolves the fix-session model via the linked integration's stored bot model with a provenance check and zod-validated fallback; logic, authorization boundary, and test coverage are sound.

Files Reviewed (4 files)
  • apps/web/src/app/cloud-agent-fork/review/[reviewId]/route.ts
  • apps/web/src/app/cloud-agent-fork/review/[reviewId]/route.test.ts
  • apps/web/src/lib/bot/model.ts
  • apps/web/src/lib/bot/model.test.ts
  • apps/web/src/lib/bot/agent-runner.ts
Notes
  • The route now resolves the model from the review's platform_integration_id via getIntegrationById rather than getReviewConfig. A provenance guard rejects integrations whose platform, owned_by_user_id, or owned_by_organization_id differ from the review's, redirecting to fix_session_failed without creating a session. Since codeReviews.get already enforces the caller's access to the review, tying the integration to the review's owner is a reasonable authorization boundary and prevents cross-owner model leakage via a tampered platform_integration_id.
  • resolveBotModelSlug validates metadata.model_slug with zod (trim + min(1)) and falls back to DEFAULT_BOT_MODEL for null integration, missing/empty/non-string slugs, and non-object metadata. Strictly safer than the previous inline cast in agent-runner.ts.
  • Unlinked reviews (platform_integration_id is null) and deleted integrations (getIntegrationById returns null) both resolve to DEFAULT_BOT_MODEL and still create a session, as covered by tests.
  • No module-scope transport-owning clients or cached DB handles introduced; this is a Next.js route using the shared db, so no Worker/DO memory-leak concern.
  • getIntegrationById is called without an organizationId scope filter, but the subsequent owner-equality check enforces ownership, so no authorization gap.
Previous Review Summary (commit 3cdd59b)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 3cdd59b)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Reviewed the fix-session model-resolution change in the Cloud Agent review route and its tests; the logic is sound, both tRPC caller paths resolve correctly, and the new failure-path coverage is adequate.

Files Reviewed (2 files)
  • apps/web/src/app/cloud-agent-fork/review/[reviewId]/route.ts
  • apps/web/src/app/cloud-agent-fork/review/[reviewId]/route.test.ts
Notes
  • The route now resolves the configured catalog model via personalReviewAgent.getReviewConfig / organizations.reviewAgent.getReviewConfig instead of reusing review.model (the usage-billing snapshot). Both router paths exist in the root router and modelSlug is always returned (defaults to PRIMARY_DEFAULT_MODEL), so no null/undefined reaches prepareSession.
  • The getReviewConfig call is inside the existing try/catch, so configuration lookup failures redirect to fix_session_failed without creating a session, as covered by the new test.
  • No module-scope state or transport-owning clients are introduced; no memory-leak concern.

Reviewed by glm-5.2-20260616 · 229,866 tokens

Review guidance: REVIEW.md from base branch main

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@alex-alecu